Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

gh-107249: Implement Py_UNUSED() for MSVC #107250

Merged
merged 1 commit into from
Jul 25, 2023
Merged

gh-107249: Implement Py_UNUSED() for MSVC #107250

merged 1 commit into from
Jul 25, 2023

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Jul 25, 2023 •
edited by bedevere-bot
Loading

Fix warnings C4100 in Py_UNUSED() when Python is built with "cl /W4".

Example with this function included by Python.h:

static inline unsigned int
PyUnicode_IS_READY(PyObject* Py_UNUSED(op))
{ return 1; }

Without this change, building a C program with "cl /W4" which just includes Python.h emits the warning:

Include\cpython/unicodeobject.h(199):
warning C4100: '_unused_op': unreferenced formal parameter

This change fix this warning.

@vstinner
pythongh-107249: Implement Py_UNUSED() for MSVC
b9cc3b6 
Fix warnings C4100 in Py_UNUSED() when Python is built with "cl /W4".

Example with this function included by Python.h:

    static inline unsigned int
    PyUnicode_IS_READY(PyObject* Py_UNUSED(op))
    { return 1; }

Without this change, building a C program with "cl /W4" which just
includes Python.h emits the warning:

    Include\cpython/unicodeobject.h(199):
    warning C4100: '_unused_op': unreferenced formal parameter

This change fix this warning.
@bedevere-bot bedevere-bot mentioned this pull request Jul 25, 2023
Closed
@vstinner
Copy link
Member Author

@zooba @serhiy-storchaka @methane: Do you want to trade a review? I consider that it's a feature change, so it requires to be approved to be merged, following new Steering Council guidelines :-)

@vstinner
Copy link
Member Author

Do you think that it would too late to backport it to Python 3.12? We are close to Python 3.12.0 beta 4 (Tuesday, 2023-07-11). I'm not sure if it's a bugfix or a feature :-/ But I would prefer to not backport it to Python 3.11 since I'm not 100% sure that my solution works in all cases on all MSVC versions.

@methane
Copy link
Member

methane commented Jul 25, 2023

But I would prefer to not backport it to Python 3.11 since I'm not 100% sure that my solution works in all cases on all MSVC versions.

+1.

methane
methane approved these changes Jul 25, 2023
View reviewed changes
Copy link
Member

@methane methane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(void)name; in function name might be the simplest solution.
But existing Py_UNUSED() macro is used in function signature.
So I don't know better approach than this PR.

@vstinner vstinner merged commit 6a43cce into python:main Jul 25, 2023
@vstinner vstinner deleted the py_unused_msvc branch July 25, 2023 17:28
@vstinner
Copy link
Member Author

Thanks for the review @methane. I merged my PR.

(void)name; in function name might be the simplest solution

Is it possible to put ; in the argument list? int test(int arg) { return 0; } would become int test(int (void)arg;) { return 0; }?

Well, Python 3.12 is already at beta stage. I prefer to not cause any regression. Let's wait until Python 3.13 is shipped. I don't think that this change is so important that it deserves to be backported.

@vstinner vstinner mentioned this pull request Jul 25, 2023
Closed
@serhiy-storchaka
Copy link
Member

C++17 supports

int test([[maybe_unused]] int arg)

I am not sure, but maybe it can be also written as

int test(int [[maybe_unused]] arg)

or

int test(int arg [[maybe_unused]])

If it is so, it can be utilized by Py_UNUSED() in extensions written in C++17.

jtcave pushed a commit to jtcave/cpython that referenced this pull request Jul 27, 2023
@vstinner @jtcave
pythongh-107249: Implement Py_UNUSED() for MSVC (python#107250)
f7f712e 
Fix warnings C4100 in Py_UNUSED() when Python is built with "cl /W4".

Example with this function included by Python.h:

    static inline unsigned int
    PyUnicode_IS_READY(PyObject* Py_UNUSED(op))
    { return 1; }

Without this change, building a C program with "cl /W4" which just
includes Python.h emits the warning:

    Include\cpython/unicodeobject.h(199):
    warning C4100: '_unused_op': unreferenced formal parameter

This change fix this warning.
@vstinner vstinner added the needs backport to 3.12 bug and security fixes label Dec 13, 2024
@miss-islington-app
Copy link

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 13, 2024
@vstinner @miss-islington
pythongh-107249: Implement Py_UNUSED() for MSVC (pythonGH-107250)
3e904d1 
Fix warnings C4100 in Py_UNUSED() when Python is built with "cl /W4".

Example with this function included by Python.h:

    static inline unsigned int
    PyUnicode_IS_READY(PyObject* Py_UNUSED(op))
    { return 1; }

Without this change, building a C program with "cl /W4" which just
includes Python.h emits the warning:

    Include\cpython/unicodeobject.h(199):
    warning C4100: '_unused_op': unreferenced formal parameter

This change fix this warning.
(cherry picked from commit 6a43cce)

Co-authored-by: Victor Stinner <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Dec 13, 2024

GH-127907 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 bug and security fixes label Dec 13, 2024
@vstinner
Copy link
Member Author

GH-127907 is a backport of this pull request to the 3.12 branch.

I need this to backport test_cext to 3.12: issue gh-127906.

vstinner added a commit that referenced this pull request Dec 13, 2024
@miss-islington @vstinner
[3.12] gh-107249: Implement Py_UNUSED() for MSVC (GH-107250) (#127907)
f3a689f 
gh-107249: Implement Py_UNUSED() for MSVC (GH-107250)

Fix warnings C4100 in Py_UNUSED() when Python is built with "cl /W4".

Example with this function included by Python.h:

    static inline unsigned int
    PyUnicode_IS_READY(PyObject* Py_UNUSED(op))
    { return 1; }

Without this change, building a C program with "cl /W4" which just
includes Python.h emits the warning:

    Include\cpython/unicodeobject.h(199):
    warning C4100: '_unused_op': unreferenced formal parameter

This change fix this warning.
(cherry picked from commit 6a43cce)

Co-authored-by: Victor Stinner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@methane methane methane approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@vstinner @methane @serhiy-storchaka @bedevere-bot